-
Notifications
You must be signed in to change notification settings - Fork 10
Add fieldRef #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fieldRef #74
Conversation
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
Signed-off-by: Steven Borrelli <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR #74 Test Report: Add fieldRef Feature to shellEnvVars
Summary
This report documents testing of PR #74, which adds fieldRef functionality to shellEnvVars in the function-shell project. The feature allows dynamic environment variable population from Crossplane resource fields with optional policies and default values.
Test Environment
- Function Version:
xpkg.upbound.io/upbound/function-shell:v0.7.0 - Test Branch:
pr-74 - Kubernetes Cluster: Local development cluster
- Testing Approach: Both local development mode and in-cluster deployment
Test Results Summary
✅ All tests passed successfully
- Local unit tests: 11/11 passed
- Local integration tests: 4/4 scenarios validated
- In-cluster deployment: Successfully deployed and functional
- End-to-end testing: fieldRef features working correctly
Detailed Test Results
1. Unit Test Results
=== RUN TestFromFieldRef
=== RUN TestFromFieldRef/FromContextMissingFieldOptionalPolicyDefaultValue
=== RUN TestFromFieldRef/FromCompositeValid
=== RUN TestFromFieldRef/FromCompositeMissingError
=== RUN TestFromFieldRef/FromCompositeMissingFieldOptionalPolicyDefaultValue
=== RUN TestFromFieldRef/FromCompositeMissingFieldOptionalPolicyNoDefaultValue
=== RUN TestFromFieldRef/FromContextMissingError
=== RUN TestFromFieldRef/FromContextMissingErrorDefaultPolicy
=== RUN TestFromFieldRef/FromCompositeMissingErrorDefaultPolicy
=== RUN TestFromFieldRef/FromContextValid
=== RUN TestFromFieldRef/FromContextMissingFieldOptionalPolicyNoDefaultValue
--- PASS: TestFromFieldRef (0.00s)
=== RUN TestRunFunction
--- PASS: TestRunFunction (0.03s)
--- PASS: TestRunFunction/ResponseIsEchoEnvVarFieldRefDefaultValue (0.00s)
PASS
ok github.com/crossplane-contrib/function-shell 0.367sResult: All 11 unit tests pass, including coverage of fieldRef scenarios.
2. Local Integration Testing
Test Case 1: Optional Field with Default Value
Input XR:
spec:
field: "hello from the XR"
# optional field intentionally omittedExpected Result: Use composition default value
Actual Result: ✅
echo: hello from the XR optional: defaultValue from Composition
Test Case 2: Optional Field with Actual Value
Input XR:
spec:
field: "hello from the XR"
optional: "optional from the XR"Expected Result: Use actual field value
Actual Result: ✅
echo: hello from the XR optional: optional from the XR
Test Case 3: Status Field Reference
Input XR with pre-populated status:
status:
atFunction:
shell:
stdout: "previous output from shell"Expected Result: Conditional logic based on status field
Actual Result: ✅
echo: hello from the XR optional: optional from the XR
status.atFunction.shell.stdout is populated
Test Case 4: Required Field Error Handling
Input: Missing required field spec.nonexistent
Expected Result: Error with descriptive message
Actual Result: ✅
Error: cannot get observed composite value at spec.nonexistent: spec.nonexistent: no such field
3. In-Cluster Deployment Testing
Deployment
- Result: ✅ Function successfully deployed and healthy
End-to-End Validation
Final In-Cluster Test Result:
status:
atFunction:
shell:
stderr: ""
stdout: |-
echo: hello from the XR optional: defaultValue from Composition
status.atFunction.shell.stdout is populated
conditions:
- reason: ReconcileSuccess
status: "True"
type: Synced
- reason: Available
status: "True"
type: ReadyValidation Points:
- ✅ fieldRef resolving
spec.fieldcorrectly - ✅ Optional policy using default value for missing
spec.optional - ✅ Status field reference for conditional logic
- ✅ Function executing successfully with ReconcileSuccess
- ✅ XR marked as Ready and Available
Feature Capabilities Verified
Core fieldRef Functionality
- ✅ Field Path Resolution:
fieldRef.pathcorrectly resolves nested field paths - ✅ Optional Policy:
fieldRef.policy: Optionalgracefully handles missing fields - ✅ Required Policy:
fieldRef.policy: Requiredprovides strict validation with clear error messages - ✅ Default Values:
fieldRef.defaultValueprovides fallback values when fields are missing - ✅ Context Support: Function supports both composite resource and context field references
- ✅ Type Safety: Proper JSON marshalling/unmarshalling with correct Go struct tags
Integration Points
- ✅ Composition Integration: fieldRef works in Crossplane compositions
- ✅ Pipeline Mode: Function operates correctly in Crossplane pipeline mode
- ✅ Crossplane Runtime: Compatible with Crossplane function SDK and runtime
Error Handling & Edge Cases
- ✅ Missing Required Fields: Clear error messages with field path information
- ✅ Empty Field Paths: Validation prevents empty path configurations
- ✅ Invalid Field Paths: Proper error handling for non-existent paths
- ✅ Policy Validation: Enum validation for Optional/Required policies
- ✅ Default Value Handling: Empty string defaults work correctly
Code Quality Assessment
Implementation Quality
- Go Code: Well-structured with proper error handling and type safety
- CRD Schema: OpenAPI v3 schema with validation rules
- Test Coverage: Unit test coverage
- Documentation: Clear field descriptions and usage examples
API Design
- Consistency: fieldRef design follows Kubernetes patterns (similar to fieldRef in pods)
- Flexibility: Supports both optional and required field resolution
- Extensibility: Design allows for future enhancements without breaking changes
Recommendation
✅ APPROVE FOR MERGE
PR #74 successfully implements the fieldRef feature with:
- Functionality covering specified requirements
- Error handling with clear error messages
- Test coverage including unit, integration, and end-to-end tests
- Production readiness validated in-cluster deployment
- Clean implementation following Kubernetes patterns and best practices
The feature enhances the function-shell capabilities by enabling dynamic environment variable population from Crossplane resource fields, which is essential for building flexible and reusable compositions.
Test Artifacts
Generated Files
- Clean CRD schema:
package/input/shell.fn.crossplane.io_parameters.yaml - Updated Go types:
input/v1alpha1/parameters.go - Test configurations: Multiple test YAML files for various scenarios
Package Information
- Test Package:
xpkg.upbound.io/...test-org.../function-shell:v0.7.0 - Build Method: Complete clean rebuild
- Deployment Status: Successfully deployed and operational in Kubernetes cluster
Report Generated: August 18, 2025
Tested By: Claude Code Assistant and Markus
Test Duration: Multi-stage testing over multiple iterations
Description of your changes
Adds a
fieldReftoshellEnvVars. AfieldRefis like avalueRef, but it allows the ability to return a null string or adefaultValueif the field is not present.Fixes #75
An image is available for testing at
index.docker.io/steve/function-shell:fieldref.Testing
Ran the example/functionRef composition in a cluster. It correctly sets default fields and reads from the XR status:
apiVersion: example.crossplane.io/v1 kind: XR metadata: annotations: kubectl.kubernetes.io/last-applied-configuration: | {"apiVersion":"example.crossplane.io/v1","kind":"XR","metadata":{"annotations":{},"name":"example-echo"},"spec":{"field":"hello from the XR"}} creationTimestamp: "2025-08-13T16:16:54Z" finalizers: - composite.apiextensions.crossplane.io generation: 5 labels: crossplane.io/composite: example-echo name: example-echo resourceVersion: "62783" uid: c5439354-df24-4494-af80-65375852ea03 spec: compositionRef: name: shell-example compositionRevisionRef: name: shell-example-c375c75 compositionUpdatePolicy: Automatic field: hello from the XR resourceRefs: [] status: atFunction: shell: stderr: "" stdout: |- echo: hello from the XR optional: defaultValue from Composition status.atFunction.shell.stdout is populated conditions: - lastTransitionTime: "2025-08-13T16:18:14Z" observedGeneration: 5 reason: ReconcileSuccess status: "True" type: Synced - lastTransitionTime: "2025-08-13T16:18:14Z" observedGeneration: 5 reason: Available status: "True" type: ReadyI have:
contribution process. Also see docs.